Skip to content
This repository has been archived by the owner on May 9, 2018. It is now read-only.

Include all header files in podspec #658

Merged
merged 1 commit into from
Jul 24, 2015
Merged

Include all header files in podspec #658

merged 1 commit into from
Jul 24, 2015

Conversation

friedbunny
Copy link
Contributor

With CocoaPods 0.38 and SDK 0.6.1, we switched to explicitly, selectively listing our public headers in the podspec. This caused problems, because we need every header to successfully compile the project.

Rather than maintain a long list of every header, this PR includes all the headers in MapView/Map/*.h.

Fixes #631, #636

/cc @incanus

@incanus
Copy link
Contributor

incanus commented Jul 21, 2015

We might just consider backing out c47e854, which introduced this property in the last release. What is the intention of it in CocoaPods, and how do other projects use it?

Also of note: Mapbox.h contains our "public" headers, which are classes that we endorse, document, and support, as opposed to all of the headers that came from upstream.

@friedbunny friedbunny modified the milestone: 1.6.2 Jul 22, 2015
@friedbunny
Copy link
Contributor Author

Reverting c47e854 has the same effect as this PR (e.g., including all headers), except that it will be less obvious where the headers are coming from.

The official intention is:

These are the headers that will be exposed to the user’s project and from which documentation will be generated. If no public headers are specified then all the headers are considered public.

In theory this sounds fine: exclude headers without public API. But our SDK won't compile without these "private" headers. Frankly, I don't understand CocoaPod's implementation/mindset here.

Most other projects use a similar wildcard to what I'm proposing in this PR. AFNetworking, for example: s.public_header_files = 'AFNetworking/*.h'

@incanus
Copy link
Contributor

incanus commented Jul 22, 2015

These are the headers that will be exposed to the user’s project and from which documentation will be generated.

The problem here is CocoaPods trying to be "helpful" and auto-generate documentation that goes here:

http://cocoadocs.org/docsets/Mapbox-iOS-SDK/1.6.1/

In past, I've lobbied (successfully) to get a directive that links to an external place (like our hosted docs) because CocoaDocs was both putting up docs for classes we didn't want up as well as docking us points for not having all classes documented.

It seems though that CocoaDocs is still generating a page for us, even if it's not linked in the summary. So our use of public_header_files is working.

I'm worried the wildcard will mess up that page again, and people who get there will get an incomplete / inaccurate picture of the toolset.

@friedbunny friedbunny modified the milestone: 1.6.2 Jul 22, 2015
@friedbunny
Copy link
Contributor Author

The cocoapods-headers-yupyup branch adds the header.dir property to the podspec, which is: "The directory where to store the headers files so they don't break includes."

That's all it says. Setting header.dir fixes our compilation errors, but only once you pod update after you've just pod install'd.

@friedbunny
Copy link
Contributor Author

My inclination here is to go with the wildcard (or reversion) — the library compiling successfully should be our main goal. That CocoaPods is generating docs for us that are wrong and that we can't correct is regrettable, but the lessor evil in this case.

I'll post on the CocoaPods repo about these issues and see what comes of it.

@incanus
Copy link
Contributor

incanus commented Jul 23, 2015

Sounds good @friedbunny — that seems to be the right call. We do link out properly to our own docs page, so let's just go with that and not spend too many cycles on hiding/removing the CocoaDocs version since we'll deprecate before much longer anyway.

friedbunny added a commit that referenced this pull request Jul 24, 2015
Include all header files in podspec
@friedbunny friedbunny merged commit 345d6d3 into develop Jul 24, 2015
@friedbunny friedbunny deleted the liberal-podspec branch July 24, 2015 14:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants